feat(cli-push-runner): Phase c MVP — pre-push lint-screen step (default OFF)#132
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
📝 Walkthrough概要PR は Diff に対して外部 lint-screen 分類器を実行し、マークダウン形式のレポートを生成する新しいパイプラインステージを追加します。このステージは設定で無効化でき、push パイプラインをブロックしません。 変更内容Lint-Screen Feature
推定コードレビュー難度🎯 3 (中程度) | ⏱️ ~20 分 関連する可能性のある PR
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/cli-push-runner/src/config.rs (1)
113-227: ⚡ Quick win
[lint_screen]セクションのパース動作を検証するテストが不在
config_parses_with_diffが[diff]セクションのパースを検証しているように、[lint_screen]セクション全フィールドのパースをテストするケースがない。serde の field 名と TOML キー名は完全一致が必要なため、フィールドのリネームや追加が silent な None へのフォールバックに化けるリスクを防ぐためにも追加が望ましい。🧪 テスト追加の例
+ #[test] + fn config_parses_lint_screen_section() { + let toml_str = r#" +[quality_gate] +[[quality_gate.groups]] +name = "test" +commands = ["echo ok"] + +[takt] +workflow = "w" +task = "t" + +[push] +command = "echo push" + +[lint_screen] +enabled = true +exe_path = "custom/classifier.exe" +model = "mistral:7b" +endpoint = "http://localhost:11434" +timeout_secs = 30 +max_diff_lines = 1000 +output_path = ".takt/custom-report.md" +"#; + let config: Config = toml::from_str(toml_str).unwrap(); + let ls = config.lint_screen.unwrap(); + assert!(ls.enabled); + assert_eq!(ls.exe_path.as_deref(), Some("custom/classifier.exe")); + assert_eq!(ls.model.as_deref(), Some("mistral:7b")); + assert_eq!(ls.endpoint.as_deref(), Some("http://localhost:11434")); + assert_eq!(ls.timeout_secs, Some(30)); + assert_eq!(ls.max_diff_lines, Some(1000)); + assert_eq!(ls.output_path.as_deref(), Some(".takt/custom-report.md")); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli-push-runner/src/config.rs` around lines 113 - 227, Add a new unit test (similar to config_parses_with_diff) named e.g. config_parses_lint_screen that deserializes a TOML string containing a full [lint_screen] section and asserts every field on Config.lint_screen (or the LintScreen type) is Some/equals the expected values; locate the Config struct and existing tests (e.g. config_parses_with_diff, config_parses_full_without_diff) to mirror style, ensure you check optional fields unwrap_or defaults where applicable and assert that no field silently remains None due to a naming mismatch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cli-push-runner/src/stages/lint_screen.rs`:
- Around line 199-216: The table-building code in render_findings_table leaves s
(severity), r (rule), file and line unescaped which can break the Markdown
table; wrap each of those values with sanitize_cell before formatting (i.e. pass
s, r, file and the computed line string through sanitize_cell) and ensure the
line value is produced as a deterministic string (use the existing
map/unwrap_or_else flow but sanitize its result) so all four fields are escaped
consistently alongside issue and suggestion.
---
Nitpick comments:
In `@src/cli-push-runner/src/config.rs`:
- Around line 113-227: Add a new unit test (similar to config_parses_with_diff)
named e.g. config_parses_lint_screen that deserializes a TOML string containing
a full [lint_screen] section and asserts every field on Config.lint_screen (or
the LintScreen type) is Some/equals the expected values; locate the Config
struct and existing tests (e.g. config_parses_with_diff,
config_parses_full_without_diff) to mirror style, ensure you check optional
fields unwrap_or defaults where applicable and assert that no field silently
remains None due to a naming mismatch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c4d07d95-a1ce-45dd-92db-be62e6706fff
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.takt/facets/instructions/review-simplicity.mdpush-runner-config.tomlsrc/cli-push-runner/Cargo.tomlsrc/cli-push-runner/src/config.rssrc/cli-push-runner/src/main.rssrc/cli-push-runner/src/stages/lint_screen.rssrc/cli-push-runner/src/stages/mod.rs
…lt OFF, gating なし、fallback graceful)
73903d7 to
8416d3e
Compare
…#134) - todo6.md / todo-summary.md に Bundle j 3 タスク追加 (順位 94-96) - 94: docs/ 内 ../docs/ 相対パストラップ検出 lint rule (Tier 1, S) - 95: docs/todo*.md preamble file count 自動照合 (Tier 2, S) - 96: Markdown cross-reference validator CI step (Tier 2, M) - local-llm-offload-analysis.md に Phase c MVP 完了 (PR #132) を反映 - 実装方針変更 (takt facet -> cli-push-runner stage) を §1 Phase c に記述 - Phase c+ (Bundle i) として PR #132 post-merge-feedback 採用 3 件 - §8.E を MVP land 済みステータスに更新 - 再開チェックリストに Phase c smoke / Bundle i 着手手順を追加 - todo-summary.md retrospective に Bundle j (PR #133 post-merge-feedback) 追記
…必須 follow-up (順位 91 + 92 + 93) (#135) * feat(cli-finding-classifier, cli-push-runner): Bundle i — Phase d 着手前必須 follow-up (順位 91 + 92 + 93) PR #132 (Phase c MVP land) の post-merge-feedback で採用された 3 件を 1 PR にまとめて land。 ## 順位 91 (Tier 2 #4): [lint_screen] config parse test - src/cli-push-runner/src/config.rs に 5 tests を追加 - silent field rename / 追加で None fallback する failure mode を unit test で防止 - full fields / minimal only enabled / absent yields None / numeric defaults / string defaults の 5 軸独立検証 ## 順位 92 (Tier 2 #5): scale-aware eval fixtures (200+ 行) - eval13-large-refactor-real.diff (5 file / 280 行) — context 限界 + JSON 完全性 - eval14-mid-mixed.diff (3 file / 153 行) — mid-scale recall 安定性 - eval15-syntax-stress.diff (1 file / 208 行) — 単 file 長尺の schema 完全性 - lint-screen-evals.json に id 13/14/15 baseline (auto_fix lane × 13 findings 合計) 追加 - count test を rename + 上限緩和 (eval_set_loads_and_has_at_least_phase_b_prime_baseline_count) - Bundle i 実体スモーク test (eval_set_includes_bundle_i_scale_aware_fixtures) 追加 ### dogfood 結果 (mistral:7b / temperature=0) agreement = 11/15 = 73.3% (Phase b' 75% から marginal 劣化 = fixture が設計通り failure mode を再現) eval13 (280 行): JSON parse error 'missing field screen_decision' → fallback path 作動 = PR #132 smoke (868 行 diff) で観測した failure mode を decisive に再現 eval15 (208 行): JSON parse error 'missing field severity at line 38' = nested field omission の別 failure mode を新規捕捉 eval14 (153 行): JSON 完全だが recall 33% (3 baseline 中 1 件のみ TP) aggregate precision=76.2% recall=51.6% latency p50=4591ms p95=8370ms verdict CONDITIONAL-GO agreement < 75% 未達理由は eval13/15 の fallback (= fixture が設計通り作動した結果) で mechanical に説明可能。Phase d 投入前の必須 measurement を取得 (todo6.md L164 「未達理由が 文書化される」branch を満たす)。§8.D v4 prompt 改訂は別 bundle に切り出し。 ## 順位 93 (Tier 3 #8): coding-style.md partial fix anti-pattern codify - ~/.claude/rules/common/coding-style.md § Cross-File Reference Lifecycle に 「変更差分外への partial fix 再発」anti-pattern を追加 - PR #94 / #111 / #132 を inline cite (実証ベース) - family_tag を grep -rn で全 path 検索する対処手順、partial fix の意図的切り出しを明記 ## Phase d 着手の前提条件 update Bundle i land で以下が揃った: - (a) [lint_screen] config silent failure 防止 (順位 91) - (b) scale-aware fixtures による failure mode の reproducible measurement (順位 92) - (c) cross-file partial fix anti-pattern の global rule 化 (順位 93) 次は §8.D v4 prompt 改訂で大規模 diff の JSON 完全性を改善するループ (Phase d 着手前の最終 gate)。 * fix(cli-finding-classifier): CodeRabbit Major #r3213115045 — eval count 下限を Bundle i baseline 15 に固定 >=12 だと既存 fixture 削除を検出できないため >= 15 に変更し regression 防止。 将来の fixture 追加 (>15) は許容。
Summary
Phase c MVP —
cli-push-runnerに lint-screen step を追加し、pre-push 時に diff を mistral:7b に流して lint 一次フィルタの所見 (.takt/lint-screen-report.md) を出力する経路を整備。Phase b' conditional GO (75%) のため gating せず、report 出力のみ (default OFF / 試験運用 opt-in)。アーキテクチャ
cli-push-runner(Rust) の新 stagestages/lint_screen.rs[lint_screen] enabled = true時のみ (default OFF)cli-finding-classifier.exe --mode lint-screen(stdin に diff、stdout に LintScreenResult JSON).takt/lint-screen-report.md(markdown table)review-simplicity.mdinstruction が同 path を optional context として読む (advisory 扱い)fallback 設計 (push を block しない)
human_review + fallback_reason、report に出力smoke dogfood 結果
868 行の現実 PR diff (Phase c 自身) を流したところ:
= mistral:7b は大規模 diff で JSON 構造を欠落させがちと判明。fallback path が graceful に処理。Phase d (PR-based dogfood) で頻度・対策方針を観測。
副次対応
step_timeout = 120 → 180(Phase b' end-to-end test の cargo test --ignored が境界 117s で timeout していた、commit73903d7で実証)Test plan
.takt/lint-screen-report.mdを context として認識するか (Phase d で観測)Out of scope (PR-B 行き)
Summary by CodeRabbit
リリースノート
新機能
ドキュメント
Chores